Skip to content

Conversation

@pkanoongo
Copy link
Contributor

@pkanoongo pkanoongo commented Oct 20, 2025

🔒 Security Fix: Shell Injection Prevention

⚠️ This PR is in DRAFT status - Please review and manually mark as ready when appropriate.

Issue

Fixes shell injection vulnerability identified in Jira ticket SEC-4316.

Semgrep Finding: yaml.github-actions.security.run-shell-injection.run-shell-injection

Affected Files:

  • release-poetry-package/action.yaml (lines 82-93, 104, 109)

What Changed

Modified GitHub Actions workflow file to prevent shell injection by using intermediate environment variables instead of direct interpolation of GitHub expressions in shell scripts.

Initial Fix:

  • Fixed lines 82-93: Added env vars for NEW_RELEASE_VERSION, GIT_USER_NAME, GIT_USER_EMAIL

Additional Fix (after Semgrep scan):

  • Fixed line 104: ${{ inputs.dry-run }} → env var DRY_RUN
  • Fixed line 109: ${{ steps.version.outputs.new_release_version }} → use existing env var NEW_RELEASE_VERSION
  • Total vulnerabilities fixed: 3

Pattern Applied:

  • Before (Vulnerable): run: | poetry version "${{ steps.version.outputs.new_release_version }}"
  • After (Secure):
    env:
      NEW_RELEASE_VERSION: ${{ steps.version.outputs.new_release_version }}
      DRY_RUN: ${{ inputs.dry-run }}
    run: |
      poetry version "$NEW_RELEASE_VERSION"
      if [[ "$DRY_RUN" == "false" ]]; then
        git push
      fi

Security Context

Direct interpolation of GitHub expressions (${{ ... }}) in shell scripts can lead to command injection if the input contains shell metacharacters. By using environment variables as intermediaries, we ensure inputs are treated as data rather than executable code.

Reference: GitHub Actions Security Hardening

Testing

  • ✅ Pre-commit hooks passed (all commits)
  • ✅ No functional changes - purely defensive security hardening
  • ✅ Additional Semgrep scan performed to catch any remaining issues
  • 🔍 Requires manual review and testing in your CI/CD pipeline

Review Checklist

  • Verify all interpolations are now using environment variables
  • Confirm no breaking changes to workflow functionality
  • Test in a non-production environment if possible
  • Mark PR as ready for review when satisfied

Comment on lines 86 to 100
run: |
# Use poetry to bump the version in pyproject.toml
poetry version "${{ steps.version.outputs.new_release_version }}"
poetry version "$NEW_RELEASE_VERSION"
# Specify the user name and email which is required to commit with a token auth
git config user.email "${{ inputs.git-user-name }}"
git config user.name "${{ inputs.git-user-email }}"
git config user.email "$GIT_USER_NAME"
git config user.name "$GIT_USER_EMAIL"
# Commit the bumped project version with a non-semver chore: commit message
git add pyproject.toml
git --no-pager diff --staged
git commit --author="${{ inputs.git-user-name }} <${{ inputs.git-user-email }}>" -m "chore: release ${{ steps.version.outputs.new_release_version }}" -m "[skip actions]"
git commit --author="$GIT_USER_NAME <$GIT_USER_EMAIL>" -m "chore: release $NEW_RELEASE_VERSION" -m "[skip actions]"
# Push the changes to the current (should be main) branch, if this is not a dry-run
# TODO: This might not be super safe, we should consider that this could
Copy link

@semgrep-app semgrep-app bot Oct 20, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using variable interpolation ${{...}} with github context data in a run: step could allow an attacker to inject their own code into the runner. This would allow them to steal secrets and code. github context data can have arbitrary user input and should be treated as untrusted. Instead, use an intermediate environment variable with env: to store the data and use the environment variable in the run: script. Be sure to use double-quotes the environment variable, like this: "$ENVVAR".

🍰 Fixed in commit d1c28c8 🍰

- Add env var for inputs.dry-run
- Fix lines 104 and 109 with proper env variable usage
- Semgrep findings: yaml.github-actions.security.run-shell-injection

Jira: SEC-4316
@github-actions
Copy link

Release notes preview

Below is a preview of the release notes if your PR gets merged.


2.4.2 (2025-10-21)

Bug Fixes

  • additional shell injection vulnerabilities in version update step (d1c28c8)
  • prevent shell injection in GitHub Actions workflows - SEC-4316 (ad8c55b)

@pkanoongo pkanoongo marked this pull request as ready for review October 21, 2025 15:46
@pkanoongo pkanoongo requested review from bilals12 and tagoro9 October 21, 2025 15:59
@pkanoongo pkanoongo merged commit b00b9ae into main Oct 21, 2025
4 checks passed
@pkanoongo pkanoongo deleted the fix/semgrep-shell-injection-sec-4316 branch October 21, 2025 16:47
@github-actions
Copy link

🎉 This PR is included in version 2.4.2 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants